-
-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
first implementation of sql_auditing #1367
Conversation
I don't know what went wrong with formatting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- the feature is not production ready
- formatting is broken, it has to be fixed
dev/sql_auditing.h
Outdated
} | ||
} | ||
inline static sql_auditor& auditor() { | ||
static sql_auditor auditor{"sql_auditor.txt"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if somebody wants to store logs not in sql_auditor.txt
but somewhere else?
dev/sql_auditing.h
Outdated
std::tm local_time = *std::localtime(&now_time); | ||
|
||
// Print the local time in a human-readable format | ||
auditor().log_file << "@: " << std::put_time(&local_time, "%Y-%m-%d %H:%M:%S") // Custom format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if somebody wants to use different date format? how devs who already has working log system would integrate this logic into their systems?
dev/sql_auditing.h
Outdated
} | ||
} | ||
inline static sql_auditor& auditor() { | ||
static sql_auditor auditor{"sql_auditor.txt"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if the process which runs this code doesn't have file permissions to sql_auditor.txt
especially or to write in the folder where it is located at all? it will start throwingstd::system_error{sqlite_orm::orm_error_code::failure_to_init_logfile}
even though it worked well before this PR
@@ -61,6 +62,7 @@ namespace sqlite_orm { | |||
if(sqlite3_prepare_v2(db, query.c_str(), -1, &stmt, nullptr) != SQLITE_OK) { | |||
throw_translated_sqlite_error(db); | |||
} | |||
sql_auditor::log(query); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logs are enabled always even though devs don't need them?
What’s wrong with the formatting?I followed your contribution guidelines JuanSent from my iPhoneOn 17 Dec 2024, at 4:08 AM, Yevgeniy Zakharov ***@***.***> wrote:
@fnc12 requested changes on this pull request.
the feature is not production ready
formatting is broken, it has to be fixed
In dev/sql_auditing.h:
+#include <chrono>
+#include <ctime> // For std::localtime and std::tm
+#include <iomanip> // For std::put_time
+#include <string>
+
+class sql_auditor {
+ std::ofstream log_file;
+ sql_auditor(std::string path) {
+ using namespace std;
+ log_file.open(path, ios::trunc | ios::out);
+ if(!log_file.good()) {
+ throw std::system_error{sqlite_orm::orm_error_code::failure_to_init_logfile};
+ }
+ }
+ inline static sql_auditor& auditor() {
+ static sql_auditor auditor{"sql_auditor.txt"};
what if somebody wants to store logs not in sql_auditor.txt but somewhere else?
In dev/sql_auditing.h:
+ return auditor;
+ }
+
+ public:
+ static void log(const std::string& message) {
+ // would use format if C++ 20
+ auto now = std::chrono::system_clock::now();
+
+ std::time_t now_time = std::chrono::system_clock::to_time_t(now);
+
+ // Convert to local time (std::tm structure)
+ // WARNING: localtime is not thread safe!
+ std::tm local_time = *std::localtime(&now_time);
+
+ // Print the local time in a human-readable format
+ auditor().log_file << "@: " << std::put_time(&local_time, "%Y-%m-%d %H:%M:%S") // Custom format
what if somebody wants to use different date format? how devs who already has working log system would integrate this logic into their systems?
In dev/sql_auditing.h:
+#include <chrono>
+#include <ctime> // For std::localtime and std::tm
+#include <iomanip> // For std::put_time
+#include <string>
+
+class sql_auditor {
+ std::ofstream log_file;
+ sql_auditor(std::string path) {
+ using namespace std;
+ log_file.open(path, ios::trunc | ios::out);
+ if(!log_file.good()) {
+ throw std::system_error{sqlite_orm::orm_error_code::failure_to_init_logfile};
+ }
+ }
+ inline static sql_auditor& auditor() {
+ static sql_auditor auditor{"sql_auditor.txt"};
what if the process which runs this code doesn't have file permissions to sql_auditor.txt especially or to write in the folder where it is located at all? it will start throwingstd::system_error{sqlite_orm::orm_error_code::failure_to_init_logfile} even though it worked well before this PR
In dev/util.h:
@@ -61,6 +62,7 @@ namespace sqlite_orm {
if(sqlite3_prepare_v2(db, query.c_str(), -1, &stmt, nullptr) != SQLITE_OK) {
throw_translated_sqlite_error(db);
}
+ sql_auditor::log(query);
logs are enabled always even though devs don't need them?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
@juandent probably clang-format version is incorrect (use 17) but maybe something else. I can't say cause it happened on your machine and you have to fix it no matter the reason |
@juandent you need to fix formatting |
the error seems to be in .clang-format file |
@juandent the error is not in |
No description provided.